Correct H&E stain ordering heuristic in ExtractHEStains #8551
Conversation
The previous heuristic incorrectly assumed hematoxylin has higher red channel values than eosin, when the opposite is typically true. Replace red-channel-only comparison with more robust red-blue ratio comparison to better distinguish hematoxylin from eosin stains based on their spectral properties. - Hematoxylin (nuclear, blue): lower red/blue ratio -> first column - Eosin (cytoplasm, pink): higher red/blue ratio -> second column Update tests to reflect corrected stain ordering. Documented behaviour (first column = H, second = E) now matches the actual output. Signed-off-by: Iyassou Shimels <[email protected]>
WalkthroughThe HE stain ordering in monai/apps/pathology/transforms/stain/array.py was changed to use a red-to-blue (R/B) ratio heuristic between two candidate vectors (v_min and v_max) with an epsilon guard to avoid division-by-zero. If the hematoxylin-like vector has a smaller R/B ratio it is kept first; otherwise the two vectors are swapped when constructing the two-column “he” matrix. Tests in tests/apps/pathology/transforms/test_pathology_he_stain.py and test_pathology_he_stain_dict.py were updated to reflect the swapped stain-column orientation and adjusted concentration and normalized-image expectations. No public API signatures changed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
monai/apps/pathology/transforms/stain/array.py (1)
88-96: Use ASCII name for epsilon and make the ratio robust to sign/near-zero blue; stabilize ties.Avoid Unicode identifiers, and guard against negative/near-zero denominators and noisy ties. This reduces mis-ordering under sign flips and tiny B.
- # Hematoxylin: high blue, lower red (low R/B ratio) - # Eosin: high red, lower blue (high R/B ratio) - ε = np.finfo(np.float32).eps - v_min_rb_ratio = v_min[0, 0] / (v_min[2, 0] + ε) - v_max_rb_ratio = v_max[0, 0] / (v_max[2, 0] + ε) - if v_min_rb_ratio < v_max_rb_ratio: + # Hematoxylin: high blue, lower red (low |R|/|B| ratio) + # Eosin: high red, lower blue (high |R|/|B| ratio) + eps = np.finfo(np.float32).eps + v_min_rb_ratio = np.abs(v_min[0, 0]) / (np.abs(v_min[2, 0]) + eps) + v_max_rb_ratio = np.abs(v_max[0, 0]) / (np.abs(v_max[2, 0]) + eps) + # keep order on ties for determinism + if v_min_rb_ratio <= v_max_rb_ratio: he = np.array((v_min[:, 0], v_max[:, 0]), dtype=np.float32).T else: he = np.array((v_max[:, 0], v_min[:, 0]), dtype=np.float32).T
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (3)
monai/apps/pathology/transforms/stain/array.py(1 hunks)tests/apps/pathology/transforms/test_pathology_he_stain.py(4 hunks)tests/apps/pathology/transforms/test_pathology_he_stain_dict.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/apps/pathology/transforms/stain/array.pytests/apps/pathology/transforms/test_pathology_he_stain.pytests/apps/pathology/transforms/test_pathology_he_stain_dict.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build-docs
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: packaging
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (macOS-latest)
🔇 Additional comments (7)
tests/apps/pathology/transforms/test_pathology_he_stain_dict.py (3)
42-46: Expected stain matrix update matches the new H/E ordering heuristic.Numbers reflect lower |R|/|B| for H (first col). Good.
61-66: Normalized image expectation updated consistently with swapped columns.Looks correct given the new he ordering.
201-214: Docstring aligns with the new derivation and values.Narrative and matrices are consistent.
tests/apps/pathology/transforms/test_pathology_he_stain.py (4)
48-52: Expected stain matrix reflects H first (lower |R|/|B|).Matches the revised heuristic.
67-72: Normalized image expectation updated for the new column order.Consistent with extraction change.
136-139: Docstring example adjusted to the new ordering.Values and explanation are coherent.
207-219: Case 4 docstring and matrices updated correctly.Concentration reordering and final image values track the new H/E order.
|
Hi @bhashemian would you be able to review this change or suggest who might be better placed? Thanks! |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Signed-off-by: Bruce Hashemian <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM! Thanks @iyassou for your contribution. That was a nice improvement.
|
/build |
3 participants
Description
I noticed when attempting to extract the hematoxylin stain from an H&E stained image that the previous heuristic for ordering hematoxylin first and eosin second incorrectly assumed hematoxylin has a higher red channel value, when the opposite is typically true. I've corrected this and implemented a more robust heuristic that compares red-blue ratios instead, since:
I've updated the tests to reflect the corrected stain ordering, and the output now accurately reflects the documented behaviour (first column hematoxylin, second eosin).
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.